examples/human_input/temporal/main.py (3)

39-44: Fix docstring Args to reflect app_ctx parameter.

Docstring says “input: none” though the function accepts app_ctx. Update for clarity.

-    Args:
-        input: none
-
-    Returns:
-        str: The greeting result from the agent
+    Args:
+        app_ctx (Context): Workflow context provided by the framework; exposes app and executor.
+
+    Returns:
+        str: The greeting result from the agent.

---

31-31: Optional: make app_ctx non‑optional or guard against None.

If the framework always provides app_ctx for @app.tool, mark it non‑optional. Otherwise add a small guard before use.

Option A (preferred if guaranteed non‑None):

-async def greet(app_ctx: Context | None = None) -> str:
+async def greet(app_ctx: Context) -> str:

Option B (if None is possible):

-    app = app_ctx.app
+    if app_ctx is None:
+        raise RuntimeError("app_ctx is required in workflow context")
+    app = app_ctx.app

Please confirm which contract the decorator enforces.

Also applies to: 46-47

---

63-63: Confirm logger supports structured kw argument ‘data’.

If using Python stdlib logging, logger.info won’t accept arbitrary kwargs. If this isn’t the mcp-agent structured logger, switch to extra=… or serialize the data.

Example alternatives:

• logger.info("[workflow-mode] greet_tool agent result", extra={"data": {"result": result}})
• logger.info("[workflow-mode] greet_tool agent result %s", {"result": result})

src/mcp_agent/mcp/sampling_handler.py (2)

33-39: Use a shared content-normalizer instead of hasattr checks.

Current hasattr-based check can produce awkward strings for lists/bytes and duplicates logic with response formatting. Prefer a small helper and isinstance(TextContent).

Apply this diff within the block:

-    for i, msg in enumerate(params.messages):
-        content = msg.content.text if hasattr(msg.content, "text") else str(msg.content)
+    for i, msg in enumerate(params.messages):
+        content = _content_to_text(msg.content)

Add this helper near the top of the file (e.g., after logger definition):

def _content_to_text(content) -> str:
    if content is None:
        return ""
    if isinstance(content, TextContent):
        return content.text or ""
    if isinstance(content, (list, tuple)):
        parts = []
        for c in content:
            parts.append(_content_to_text(c))
        return " ".join(p for p in parts if p)
    if hasattr(content, "data"):
        try:
            # bytes -> utf-8, else str()
            data = content.data
            if isinstance(data, bytes):
                return data.decode("utf-8", errors="replace")
            return str(data)
        except Exception:
            return str(getattr(content, "data", content))
    if isinstance(content, bytes):
        return content.decode("utf-8", errors="replace")
    if isinstance(content, dict) and "text" in content:
        return str(content.get("text", ""))
    return str(content)

Additionally, consider truncating the preview consistently (e.g., 200 chars) via a small helper to keep prompts compact.

---

85-86: Align response formatting with request path and reuse normalizer.

Mirror the request path: normalize content via a helper and optionally truncate to prevent overly long human prompts.

Apply this diff:

-    content = (
-        result.content.text if hasattr(result.content, "text") else str(result.content)
-    )
+    content = _content_to_text(result.content)

(Optional) Truncate similarly to the request view:

preview = content[:500] + ("..." if len(content) > 500 else "")

examples/human_input/temporal/client.py (1)

17-24: Good defensive fallbacks for optional imports.

Minor nit: catch ImportError instead of broad Exception.

Apply this diff:

-try:
-    from exceptiongroup import ExceptionGroup as _ExceptionGroup  # Python 3.10 backport
-except Exception:  # pragma: no cover
+try:
+    from exceptiongroup import ExceptionGroup as _ExceptionGroup  # Python 3.10 backport
+except ImportError:  # pragma: no cover
     _ExceptionGroup = None  # type: ignore
-try:
-    from anyio import BrokenResourceError as _BrokenResourceError
-except Exception:  # pragma: no cover
+try:
+    from anyio import BrokenResourceError as _BrokenResourceError
+except ImportError:  # pragma: no cover
     _BrokenResourceError = None  # type: ignore

src/mcp_agent/server/app_server.py (2)

819-833: Ensure Temporal client is initialized before signaling.

Call ensure_client() (when available) before accessing executor.client to avoid None or uninitialized client in some worker lifecycles.

-                        if app and app.context and hasattr(app.context, "executor"):
-                            executor = app.context.executor
-                            if hasattr(executor, "client"):
-                                client = executor.client
+                        if app and app.context and hasattr(app.context, "executor"):
+                            executor = app.context.executor
+                            if hasattr(executor, "ensure_client"):
+                                await executor.ensure_client()
+                            if hasattr(executor, "client"):
+                                client = executor.client

---

526-535: Harden Bearer token parsing.

partition()+strip() tolerates extra spaces/tabs and avoids partial tokens.

-            bearer = request.headers.get("Authorization", "")
-            bearer_token = (
-                bearer.split(" ", 1)[1] if bearer.lower().startswith("bearer ") else ""
-            )
+            bearer = request.headers.get("Authorization", "")
+            scheme, _, token = bearer.partition(" ")
+            bearer_token = token.strip() if scheme.lower() == "bearer" else ""

src/mcp_agent/human_input/elicitation_handler.py (1)

102-114: Consider surfacing metadata in the response.

If result/content carries useful fields beyond “response”, optionally populate HumanInputResponse.metadata for downstream consumers.

src/mcp_agent/mcp/client_proxy.py (2)

190-206: Unbounded HTTP timeout by default.

httpx.Timeout(None) can hang indefinitely. In Temporal activities the activity timeout may bound it, but in other async contexts this can stall the system.

Apply this diff to default to a sane finite timeout (overrideable via env):

-        timeout_str = os.environ.get("MCP_GATEWAY_REQUEST_TIMEOUT")
+        timeout_str = os.environ.get("MCP_GATEWAY_REQUEST_TIMEOUT", "60")
@@
-            if timeout_float is None:
-                timeout = httpx.Timeout(None)
+            if timeout_float is None or timeout_float <= 0:
+                timeout = 60.0

Repeat a similar change in the non‑workflow branch below.

---

248-251: DRY: factor header building into a helper.

Header construction is duplicated across functions. Extract a small _auth_headers(gateway_token: Optional[str]) -> Dict[str, str] to reduce copy/paste and drift.

I can draft the helper and apply it across this file on request.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: rholinshead
PR: lastmile-ai/mcp-agent#414
File: src/mcp_agent/logging/logger.py:18-19
Timestamp: 2025-09-05T14:31:48.139Z
Learning: In the mcp-agent logging module (src/mcp_agent/logging/logger.py), temporalio should be imported lazily with try/except ImportError to avoid making it a hard dependency. Use temporalio.workflow.in_workflow() instead of isinstance checks on internal classes like _WorkflowInstanceImpl.

Learnt from: CR
PR: lastmile-ai/mcp-agent#0
File: examples/usecases/reliable_conversation/CLAUDE.md:0-0
Timestamp: 2025-07-22T18:59:49.368Z
Learning: Applies to examples/usecases/reliable_conversation/examples/reliable_conversation/src/**/*.py : Use mcp-agent's Agent abstraction for ALL LLM interactions, including quality evaluation, to ensure consistent tool access, logging, and error handling.

Learnt from: saqadri
PR: lastmile-ai/mcp-agent#386
File: src/mcp_agent/mcp/mcp_server_registry.py:110-116
Timestamp: 2025-08-28T15:07:10.015Z
Learning: In MCP server registry methods, when client_session_factory parameters are updated to accept additional context parameters, ensure the type hints match what is actually passed (Context instance vs ServerSession) and that the default factory (MCPAgentClientSession) can handle the number of arguments being passed to avoid TypeError at runtime.